feat(auth): reshape reauthContent into a ReauthContentState content slot - #2452
feat(auth): reshape reauthContent into a ReauthContentState content slot#2452demolaf wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a robust reauthentication flow in FirebaseUI Auth for Android, separating operation-level cancellations (AuthState.Cancelled) from flow-level aborts (AuthState.Aborted). It adds support for a custom, stateless reauthContent slot in FirebaseAuthScreen while keeping credential exchanges owned by the library, locks the email field to read-only during reauthentication, and resolves several state-resetting edge cases. The review feedback suggests making the OAuth reauthentication path more robust and fail-fast by explicitly throwing an exception if auth.currentUser is unexpectedly null, rather than silently failing with a safe call.
5dfbe74 to
81b3b32
Compare
1e1858f to
82c68c0
Compare
9a312b5 to
692a7a5
Compare
692a7a5 to
f05a83c
Compare
russellwheatley
left a comment
There was a problem hiding this comment.
Two suggestions on the reauth config copy and state survival across rotation, nothing blocking otherwise. The uid-matching consumption logic, the internal AuthState.Success constructor, and the inert-while-armed gating all check out.
| reauthSubRoute.value = null | ||
| reauthError.value = null | ||
| pendingReauthState.value = state | ||
| pendingReauthConfig.value = configuration.copy( |
There was a problem hiding this comment.
This copy() only overrides providers/isNewEmailAccountsAllowed/isReauthenticationMode, so isCredentialLinkingEnabled stays whatever the app set at the top level. If it's true, signInAndLinkWithCredential takes the linkWithCredential branch instead of reauthenticate for a credential that's already linked (filterToLinkedProviders only offers linked ones), which either gets rejected by Firebase or succeeds without actually satisfying the recent-login guarantee this flow exists for. Not covered by a test that combines isCredentialLinkingEnabled with isReauthenticationMode. Might be worth forcing isCredentialLinkingEnabled = false (and isAnonymousUpgradeEnabled = false for symmetry) here.
There was a problem hiding this comment.
Fixed, canLinkCredential now requires !config.isReauthenticationMode, so a reauthentication can never use linkWithCredential. Left isAnonymousUpgradeEnabled alone — anonymous users have no linked providers, so it's unreachable.
reauthContentwas documented and shaped as a content slot alongsideemailContent,phoneContentand the MFA slots, but it received only(AuthState.ReauthenticationRequired, onDismiss)and every API needed to build a reauthentication UI —filterToLinkedProviders,isReauthenticationMode, the federated provider driver — wasinternalorprivate. A custom slot could therefore only perform email/password reauthentication and had to dead-end for a Google- or OAuth-only account. The success handoff was also easy to get wrong:onDismissreset auth state toIdlewhileretryOperationemittedAuthState.Success, so both orderings a caller would naturally reach either clobbered the success or cancelled the scope the retry ran in, silently dropping the sensitive operation.reauthContentnow receives a singleReauthContentStatecarrying the user, the reason, the providers already filtered to those linked to that user, and callbacks to select a provider or dismiss. The caller renders a provider chooser; the library owns credential exchange and dismiss/retry sequencing, so federated reauthentication is expressible from a custom slot and the ordering contract is no longer the caller's problem. SelectingAuthProvider.EmailorAuthProvider.Phonehands off to the library's own sub-flow, honouring the caller'semailContent/phoneContent.reauthContenttakes a singleReauthContentStateinstead of(state, onDismiss).AuthState.Successcan no longer be constructed outside the library. It now records which uid a reauthentication re-proved, and that proof must not be forgeable by app code.ReauthContentState.kt: new public state holder, following theMfaEnrollmentContentStateconventions.FirebaseAuthScreen.kt: the already-computed linked-provider list now reaches the slot instead of being discarded; the pending operation is consumed only for a library-published success on the same uid; provider selection and error-dialog recovery are inert while reauthentication is armed.AuthState.kt:SuccessgainsreauthenticatedUidand aninternalconstructor.EmailAuthProvider+FirebaseAuthUI.kt,OAuthProvider+FirebaseAuthUI.kt: stampreauthenticatedUidwhere the reauthenticated identity is known; account creation is rejected outright in reauthentication mode.SignInUI.kt: the sign-up route is hidden in reauthentication mode, and Credential Manager autofill is skipped so a saved password for another account cannot be auto-submitted.EmailAuthScreen.kt,ResetPasswordUI.kt,SignInEmailLinkUI.kt: the reauth email screen is prefilled with the signed-in address and renders it read-only in every mode that shows it.Added
FirebaseAuthScreenReauthContentStateTestandEmailAuthScreenReauthEmailLockTest, plus e2e coverage of reauthentication through the slot — each new test verified to be load-bearing by temporarily reverting the fix and confirming it fails.Usage